Skip to content

'for' loop hygiene, etc. #15184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Jun 26, 2014
Merged

Conversation

jbclements
Copy link
Contributor

It turns out that bindings introduced by 'for' loops were not treated hygienically. The fix for this is to make the 'for' expansion more like a macro; rather than expanding sub-pieces and then assembling them, we need to rewrite the for and then call expand again on the whole thing.

This PR includes a test and the fix.

It also contains a number of other things:

  • unit tests for other forms of hygiene (currently ignored)
  • a fix for the isaac.rs macro that (it turned out) was relying on capturing
  • other miscellaneous cleanup and comments

fld.cx.expr_match(span, discrim, vec!(arm))
// why these clone()'s everywhere? I guess I'll follow the pattern....
let match_expr = fld.cx.expr_match(span, discrim, vec!(arm));
fld.fold_expr(match_expr).clone()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this compile without the .clone?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give it a try. I worry that there's some hidden mutability that makes the clone() necessary and that the tests won't catch. Perhaps I should just have faith in the type system. If this one can go, I think there are a lot of other places where it could go, as well....

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, since it's widespread sounds like that could be a separate patch to avoid blocking this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'm going to leave it alone for now.

@emberian
Copy link
Member

r=me if clone removed

@jbclements
Copy link
Contributor Author

@bors: retry

bors added a commit that referenced this pull request Jun 26, 2014
…ents

It turns out that bindings introduced by 'for' loops were not treated hygienically. The fix for this is to make the 'for' expansion more like a macro; rather than expanding sub-pieces and then assembling them, we need to rewrite the for and then call expand again on the whole thing.

This PR includes a test and the fix.

It also contains a number of other things:
- unit tests for other forms of hygiene (currently ignored)
- a fix for the isaac.rs macro that (it turned out) was relying on capturing
- other miscellaneous cleanup and comments
@bors bors closed this Jun 26, 2014
@bors bors merged commit e880c42 into rust-lang:master Jun 26, 2014
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 17, 2023
Disable mir interpreter for targets with different pointer size from host

fix rust-lang#15182
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants